Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

web/ui #1342

Merged
merged 10 commits into from
Jun 24, 2024
Merged

web/ui #1342

merged 10 commits into from
Jun 24, 2024

Conversation

TalDerei
Copy link
Contributor

@TalDerei TalDerei commented Jun 20, 2024

The idea of this PR is to prepare the UI package for extraction different components to Minifront and Prax. Here's what's done:

  • Fixed the build scripts + TS
  • Added the readme
  • Put all components into same-name directory with an index file, so it is easier to consume the components
  • Added more Storybook stories

Copy link

changeset-bot bot commented Jun 20, 2024

🦋 Changeset detected

Latest commit: e9f30c5

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@TalDerei TalDerei marked this pull request as draft June 20, 2024 07:11
@TalDerei TalDerei changed the title ui package web/ui Jun 20, 2024
Comment on lines +16 to +21
export default {
content: [
// Parses the classes of the UI components
'./node_modules/@penumbra-zone/ui/**/*.js',
],
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Info on how to consume the UI package externally. More on "why" here: prax-wallet/prax#45 (comment)

Comment on lines +37 to 40
"./components/ui/*": {
"default": "./dist/components/ui/*/index.js",
"types": "./dist/components/ui/*/index.d.ts"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now all components have their own directory and an index.tsx file – all imports should look like import {} from '@penumbra-zone/ui/components/ui/component-name.

I really wish to remove the components/ui part of the import but it would make the PR even bigger.

@@ -4,7 +4,7 @@ import { ActionViewComponent } from './action-view';
import { ViewBox, ViewSection } from './viewbox';
import { getStakingTokenMetaData } from './registry';
import { ValueView } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1/asset_pb';
import { ValueViewComponent } from './value';
import { ValueViewComponent } from '../value';

export const TransactionViewComponent = ({ txv }: { txv: TransactionView }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TransactionViewComponent is the only transaction-related component that was used by both Minifront and Prax inside the monstrous tx directory. I extracted other components and cleaned the directory up a little

Comment on lines +13 to +24
type Story = StoryObj<typeof AssetIcon>;

const EXAMPLE_METADATA = new Metadata({
base: 'upenumbra',
display: 'penumbra',
symbol: 'UM',
images: [
{
svg: 'https://raw.githubusercontent.com/prax-wallet/registry/main/images/um.svg',
},
],
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the components (still not all) have their meaningful Storybook stories

@VanishMax VanishMax self-assigned this Jun 20, 2024
@VanishMax VanishMax marked this pull request as ready for review June 20, 2024 19:09
@VanishMax VanishMax requested review from a team and turbocrime June 20, 2024 19:10
@TalDerei TalDerei mentioned this pull request Jun 21, 2024
@TalDerei
Copy link
Contributor Author

TalDerei commented Jun 23, 2024

  • Returned the package name @penumbra-zone/ui back instead of @repo/ui

based on prax-wallet/prax#45 (comment), prax won't be externally consuming the ui package, so we should revert back to using @repo/ui.

# Conflicts:
#	apps/minifront/src/components/shared/selectors/balance-selector.tsx
@VanishMax
Copy link
Contributor

@TalDerei Changed the package name back to repo/ui

@TalDerei TalDerei requested a review from VanishMax June 24, 2024 12:12
@TalDerei
Copy link
Contributor Author

TalDerei commented Jun 24, 2024

LGTM! I think this lays some really nice groundwork

prax-wallet/prax#51 is a good follow-up

@VanishMax VanishMax merged commit 24d9bfa into main Jun 24, 2024
6 checks passed
@VanishMax VanishMax deleted the ui-package branch June 24, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants